-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V3 update header #944
V3 update header #944
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are accessibility issues in these changes.
<Text type="labelRegular">{t("contact_us")}</Text> | ||
</Button> | ||
</div> | ||
<button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 You fixed the issue(s)! Great work.
/> | ||
)} | ||
<Button size="large" type="primary" background="dark"> | ||
<Text type="labelRegular">{t("contact_us")}</Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add :mailto in this button
display: flex; | ||
flex-direction: column; | ||
|
||
@media (max-width: 1024px) { | ||
background: var(--background-bg-dark); | ||
background: var(--primary-bg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a colorvariable that we have?
min-height: 5rem; | ||
padding: 0rem 3rem 0rem 3rem; | ||
border-radius: 0.75rem; | ||
background: rgba(250, 250, 250, 0.75); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace with our global variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we have lower opacity on the background, we have no global variable for the color of the header, but I can make a new variable for the header background so we can still control it from global.css!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a401d8e
There are some bugs with AccessLint. It does not refresh after it fails, even if you already fixed the access issues. For now, you can ignore the pending check. |
The header is now updated with the new design 💫
Note:
The
contact us
button does not have any functionality yet, it needs to be added before launch. Also, the languageswitch sometimes switches the order of the languages, I could not figure out why.And the announcement banner needs to be reworked
Desktop
Screen.Recording.2024-12-04.at.09.19.51.mov
Tablet and mobile
Screen.Recording.2024-12-04.at.09.22.13.mov
Scrolling
Screen.Recording.2024-12-04.at.09.23.54.mov
Closes #906 and #830